Request to add error span tag to Opentelemetry specification #86
Conversation
vikrambe
requested review from
AloisReitbauer,
bogdandrutu,
c24t,
carlosalberto,
iredelmeier,
jmacd,
reyang,
SergeyKanzhelev,
songy23,
tedsuo,
tigrannajaryan and
yurishkuro
as code owners
| ## Motivation | ||
|
|
||
| ### Why should we make this change? | ||
| Tracing backends such as jaeger adhere to opentracing spec [opentracing] (https://github.com/opentracing/specification/blob/master/semantic_conventions.md#span-tags-table). According to opentracing specification [i.e. also how jaeger has implemented] "error" span tag is a boolean value that helps in filtering spans that are in error state. Since opentelemetry does not have "error" span tag in the specification, error span tags are passed as is to the exported backend. According to opentracing spec error tag is boolean, but when zipkin span is translated to jaeger span format "error" tag continues to be a string carrying complete error message. This violates opentracing specification and also breaks jaeger functionality, as jaeger expects "error" to be boolean and "error.message" to be String type that carries description of the error message. |
There was a problem hiding this comment.
| Tracing backends such as jaeger adhere to opentracing spec [opentracing] (https://github.com/opentracing/specification/blob/master/semantic_conventions.md#span-tags-table). According to opentracing specification [i.e. also how jaeger has implemented] "error" span tag is a boolean value that helps in filtering spans that are in error state. Since opentelemetry does not have "error" span tag in the specification, error span tags are passed as is to the exported backend. According to opentracing spec error tag is boolean, but when zipkin span is translated to jaeger span format "error" tag continues to be a string carrying complete error message. This violates opentracing specification and also breaks jaeger functionality, as jaeger expects "error" to be boolean and "error.message" to be String type that carries description of the error message. | |
| Tracing backends such as jaeger adhere to opentracing spec [opentracing] (https://github.com/opentracing/specification/blob/master/semantic_conventions.md#span-tags-table). According to opentracing specification [i.e. also how jaeger has implemented] "error" span tag is a boolean value that helps in filtering spans that are in error state. Since opentelemetry does not have "error" span tag in the specification, error span tags are passed as is to the exported backend. According to opentracing spec error tag is boolean, but when zipkin span is translated to jaeger span format "error" tag continues to be a string carrying complete error message. This violates opentracing specification and also breaks jaeger functionality, as jaeger expects "error" to be boolean and "error.message" to be String type that carries description of the error message. |
Otherwise this is rendered as a single line of code with a horizontal scroll bar (same below).
Also, please add line breaks, and check the rendered PR in general, there are a few formatting typos.
There was a problem hiding this comment.
Errors are already directly representable in OpenTelemetry data model via Status field which is part of the API [1] and of the data model at the protocol level [2]:
[1] https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/api-tracing.md#set-status
[2] https://github.com/open-telemetry/opentelemetry-proto/blob/a7a235db6b05a921b06ba1deb6adcbe43343cdb6/opentelemetry/proto/trace/v1/trace.proto#L223
The Status field conveys both the fact that there is an error via Status.code field and the error message via Status.message field.
The rationale for another way of conveying errors is unclear. Is the proposal that OpenTracing's semantic convention around "error" is adopted? That would require re-considering how Status is implemented since co-existence of "error" attribute and Status code results in conflicting information (e.g. what it means if Status.code==0 and "error"=true?)
|
Status and error are overlapping, but not the same, if I interpret "true if and only if the application considers the operation represented by the Span to have failed" as correct. Best example is HTTP 404 which has a Status of NOT_FOUND, but in many situations is not really an error. |
@tigrannajaryan Jaeger uses Opentracing semantic, do we want to stick with 'Status' ? Is Status field borrowed from opencensus ? We might have to check for |
|
I would like to see a better clarification about how Status and error will play together. As of now it appears the Status field in the proto claims to be defining what is an error: If this is wrong then we need to fix this. |
|
This is a 4th option now for errors. The other three that are currently being proposed are: |
|
Not to forget the discussions on open-telemetry/opentelemetry-specification#306 |
|
This feels like a candidate for closing. The problem statement is not clear (what does conversion from zipkin to jaeger has to do with OTEL?), and there is already a prescribed mapping of OTEL status into OT error tag in the Jaeger exporter spec. |
|
@vikrambe we have re-defined the |
No description provided.